everyrow-cc-CLAUDE: Add everyrow_cancel MCP tool for task cancellation#186
everyrow-cc-CLAUDE: Add everyrow_cancel MCP tool for task cancellation#186
Conversation
Adds a new `everyrow_cancel` tool that calls the engine's
POST /tasks/{task_id}/cancel endpoint via httpx. Handles success (200),
already-terminated (409), not-found (404), and unexpected errors gracefully.
Clears task state file on both successful cancel and already-terminated cases.
Includes 6 tests covering all code paths. Phase 2 (SDK cancel method via
generated client) deferred to follow-up.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
||
| @mcp.tool(name="everyrow_cancel", structured_output=False) | ||
| async def everyrow_cancel(params: CancelInput) -> list[TextContent]: | ||
| """Cancel a running everyrow task. Use when the user wants to stop a task that is currently processing.""" |
There was a problem hiding this comment.
I would suggest moving as much of this logic to the SDK as possible and reusing that here. Then SDK users also get this feature. You'll want to regenerate interface code to reflect your API changes.
There was a problem hiding this comment.
@rgambee can you check again? I incorporated it which lead to some significant changes
src/everyrow/task.py
Outdated
| def cancel_task_sync( | ||
| task_id: UUID, client: AuthenticatedClient | ||
| ) -> None: | ||
| """Cancel a running task by its ID (synchronous). |
There was a problem hiding this comment.
Do we need a (strictly) synchronous version? The one just above this should be enough, right?
There was a problem hiding this comment.
yes I first thought we have sync functions of every op but looks like we don't
There was a problem hiding this comment.
It's confusing terminology. In one sense, all our operations are async since they're marked async def foo().
But there's another level of (a)synchrony to consider: does the function return immediately after submitting the task, or does it await the result? We use async/sync to also describe this difference. Maybe there's another pair of terms we can use instead?
These endpoints were picked up by the OpenAPI client regeneration but are not part of the forecast feature. cancel_task will be added by #186. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* feat: add forecast operation to SDK and MCP server - Add forecast() and forecast_async() to everyrow.ops - Add everyrow_forecast MCP tool with ForecastInput model - Regenerate API client from latest engine OpenAPI spec (adds forecast endpoint, list sessions, cancel task) - Add integration test for forecast operation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review — docstring, input_csv description, ruff format - Replace implementation-detail docstring with user-facing description matching SDK style (rgambee review) - Improve input_csv field description for ForecastInput (rgambee review) - Update manifest description to match new docstring - Run ruff format on generated API client files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: revert cosmetic restyle of generated API client files Revert unrelated formatting changes from openapi-python-client regeneration. Keep only the genuinely new files (forecast, cancel_task, list_sessions) and minimal __init__.py additions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: remove non-forecast generated files (list_sessions, cancel_task) These endpoints were picked up by the OpenAPI client regeneration but are not part of the forecast feature. cancel_task will be added by #186. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: extend forecast test question to July 2027 Prevents test from going stale when the resolution date passes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: rename forecast `task` to `context` and make it optional The forecast operation doesn't need a task instruction the way other ops do — a well-formed question with resolution criteria is self-contained. Rename to `context` to communicate that it provides optional batch-level framing, and make it default to None. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: update forecast MCP docstring with validation methodology Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: update forecast SDK docstring with validation methodology Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add forecast API reference and update api.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: simplify forecast reference overview paragraph Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
| except Exception as e: | ||
| return [ | ||
| TextContent( | ||
| type="text", | ||
| text=f"Error cancelling task {task_id}: {e!r}", | ||
| ) | ||
| ] |
There was a problem hiding this comment.
Bug: The generic except Exception block in everyrow_cancel fails to call _clear_task_state(), leading to stale state on unexpected API errors.
Severity: MEDIUM
Suggested Fix
Modify the generic except Exception block in the everyrow_cancel function to also call _clear_task_state(). This will ensure that the task state is consistently cleared after any cancellation attempt, regardless of whether the API call succeeds, fails with a known error, or fails with an unexpected error.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: everyrow-mcp/src/everyrow_mcp/tools.py#L792-L798
Potential issue: When the `cancel_task` API call receives an unexpected HTTP status
(e.g., 500, 503), the underlying client raises an `errors.UnexpectedStatus` exception.
This exception is not an `EveryrowError` and is caught by the generic `except Exception`
block in the `everyrow_cancel` tool. However, this exception handler does not call
`_clear_task_state()`, which is inconsistent with the handling of `EveryrowError`. This
results in the task state file (`~/.everyrow/task.json`) not being cleared, leaving
stale data and causing potential confusion about the active task's status.
Summary
everyrow_cancelMCP tool that callsPOST /api/v0/tasks/{task_id}/cancelvia httpx~/.everyrow/task.jsonon successful cancel and already-terminated casesCancelInputPydantic model with whitespace stripping and extra field rejectionTask
Backlog task:
cohort/everyrow-cc/backlog/07-task-cancellation.md(Phase 1 only)Test plan
uv run --group dev pytest tests/passes (32 passed, 5 skipped)TestCancel: running task, already terminated (409), not found (404), API error, uninitialized client, input validation🤖 Generated with Claude Code